Skip to content

Gallery's list is now fed by nsfetchedresultscontroller#607

Merged
theospears merged 6 commits intobeeminder:masterfrom
krugerk:bugfix/588-gallery-seems-slow
Jan 14, 2025
Merged

Gallery's list is now fed by nsfetchedresultscontroller#607
theospears merged 6 commits intobeeminder:masterfrom
krugerk:bugfix/588-gallery-seems-slow

Conversation

@krugerk
Copy link
Copy Markdown
Contributor

@krugerk krugerk commented Jan 5, 2025

Summary

The initial loading (displaying) of goals on the gallery was slow. The app was still relying on passing and receiving nsnotification/notifications.
With the data in Core Data, the NSFetchedResultsController can be used.

Migrated galleryVC to use the fetched results controller.
Updated the sorting implementation accordingly, directly applying the sort type to the fetchRequest itself.
Updated the filtering as well to also apply directly to the fetchRequest.

For UI changes including screenshots of before and after is great.
UI wise this looks the same, just faster/more responsive.

Validation

Ran the app in the simulator.
Changed sorting.
Changed filters.
Signed in.
Signed out.
Navigated from Gallery to Goal and back.
Sent app into background. Brought back into foreground.
Force quit app, relaunched app.

Tickets

helps with #588

@krugerk krugerk force-pushed the bugfix/588-gallery-seems-slow branch 2 times, most recently from aeea715 to ffbdc01 Compare January 8, 2025 10:34
theospears pushed a commit that referenced this pull request Jan 9, 2025
…614)

## Summary
Migrated the gallery's collection view to the more modern api, using
diffable datasource and snapshots. See
#607 (comment).

Transitions from one state to another (one set of goals to the next) is
animated.

## Validation
ran app in simulator, both iPhone and iPad (easier to see multiple
columns and animation, for example, when filtering)
rotated simulator
changed sort preference
filtered on gallery's goals
signed out
signed in
navigated from gallery to goal and back
assured that some goal's data changed (for example safeBuf and safeSum)
and the effect was seen in the cell at the same position
@krugerk krugerk force-pushed the bugfix/588-gallery-seems-slow branch from ffbdc01 to fba3fba Compare January 11, 2025 16:48
@krugerk krugerk marked this pull request as ready for review January 11, 2025 20:00
@krugerk krugerk requested a review from a team as a code owner January 11, 2025 20:00
@krugerk krugerk requested review from theospears and removed request for a team January 11, 2025 20:00
@krugerk krugerk changed the title use nsfetchedresultscontroller on the gallery Gallery's list is now fed by nsfetchedresultscontroller Jan 12, 2025
Copy link
Copy Markdown
Collaborator

@theospears theospears left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things are going to be so much cleaner without all the manual bookkeeping around goals/core data changing.

Added some notes. I think the most interesting one is around having NSFetchedResultsController create snapshots directly. I think we should at least try that, although it's possible it will end up being worse.

}
}

try? fetchedResultsController.performFetch()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Is it worth logging if this fails?

@krugerk krugerk force-pushed the bugfix/588-gallery-seems-slow branch from fedacd5 to 13d5b35 Compare January 13, 2025 19:25
@krugerk krugerk force-pushed the bugfix/588-gallery-seems-slow branch from 13d5b35 to 4999161 Compare January 13, 2025 19:29
@krugerk krugerk requested a review from theospears January 13, 2025 19:32
@theospears theospears merged commit d7a50fc into beeminder:master Jan 14, 2025
@krugerk krugerk deleted the bugfix/588-gallery-seems-slow branch January 14, 2025 08:17
theospears pushed a commit that referenced this pull request Jan 15, 2025
## Summary
A merge request for various cleanups that were kept out of #607 so as to
make for a clean diff.


## Validation
Built app
Ran it in simulator
theospears pushed a commit that referenced this pull request May 6, 2025
## Summary
When sorting the gallery by pledge or by recent data, the smallest
pledges or least recently update goals were shown first. This is in
contrast with what the website does and what the app previously did. The
app currently does not provide for easily flipping the direction of
these sorting tactics.

## Validation
Compared the gallery's order on the website with the app.


Bug was introduced in
[#607](https://github.com/beeminder/BeeSwift/pull/607/files#diff-a606a3bd59aa90e96475a1104ee2119968cb34b90d9fb33afd497f204dcb03e7L357-L363)
where the two previously "reversed" criteria were changed to ascending.
krugerk added a commit to krugerk/BeeSwift that referenced this pull request Oct 29, 2025
## Summary
When sorting the gallery by pledge or by recent data, the smallest
pledges or least recently update goals were shown first. This is in
contrast with what the website does and what the app previously did. The
app currently does not provide for easily flipping the direction of
these sorting tactics.

## Validation
Compared the gallery's order on the website with the app.


Bug was introduced in
[beeminder#607](https://github.com/beeminder/BeeSwift/pull/607/files#diff-a606a3bd59aa90e96475a1104ee2119968cb34b90d9fb33afd497f204dcb03e7L357-L363)
where the two previously "reversed" criteria were changed to ascending.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants